Skip to content

Refresh in-memory Client after sign-out and add tests#587

Open
swolfand wants to merge 2 commits intomainfrom
codex/review-android-expo-changes-for-sdk-integration
Open

Refresh in-memory Client after sign-out and add tests#587
swolfand wants to merge 2 commits intomainfrom
codex/review-android-expo-changes-for-sdk-integration

Conversation

@swolfand
Copy link
Copy Markdown
Collaborator

@swolfand swolfand commented Apr 9, 2026

Motivation

  • Ensure stale in-progress sign-in/sign-up state does not persist after sign-out by refreshing the in-memory Client while skipping the current client id.

Description

  • Add a best-effort client refresh in SignOutService.signOut() by invoking Client.getSkippingClientId() inside the finally block and logging failures.
  • Import Client and perform the refresh via runCatching { Client.getSkippingClientId() } to avoid breaking sign-out flow on refresh errors.
  • Extend tests to mock ClerkApi.client and ClientApi.getSkippingClientId() and update existing sign-out tests to include the client refresh expectation.
  • Add two new tests: signOut refreshes client after local sign-out cleanup and signOut tolerates client refresh failure which verify the client refresh is called and that refresh failures are tolerated.

Testing

  • Ran the SignOutServiceTest suite including the two new tests signOut refreshes client after local sign-out cleanup and signOut tolerates client refresh failure, and all tests passed.
  • Existing tests that verify device token deletion and session/user clearing were updated to mock ClientApi.getSkippingClientId() and passed successfully.
  • Verified that SignOutService.signOut() returns ClerkResult.Success on normal flows and still clears local state when server sign-out or client refresh fails.

Codex Task

@swolfand
fix(api): refresh client after sign-out
98f7811 
Perform a best-effort Client.getSkippingClientId() refresh after local sign-out cleanup so stale in-progress sign-in/sign-up state does not persist across remounts.\n\nAlso expands SignOutService tests to mock ClientApi and assert the refresh call occurs and failures are tolerated.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 9, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cebdd9cd-8b24-4177-964b-efdd49a932b6

📥 Commits

Reviewing files that changed from the base of the PR and between 98f7811 and 848dfd8.

📒 Files selected for processing (1)
  • source/api/src/main/kotlin/com/clerk/api/Clerk.kt
📝 Walkthrough

Walkthrough

Adds a best-effort client state refresh to the sign-out flow: after local sign-out cleanup (device token deletion and Clerk.clearSessionAndUserState()), the code attempts Client.getSkippingClientId() wrapped in runCatching and logs a warning on failure. Tests were updated to mock and verify the new client refresh behavior, including a failure case that ensures sign-out still succeeds and session/user state remains cleared. Also adds Clerk.getDeviceToken() to expose the encrypted device token (nullable).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: refreshing in-memory Client after sign-out and adding corresponding tests.
Description check ✅ Passed The description provides comprehensive motivation, implementation details, testing approach, and results. It covers the key changes and verification steps, though the template requires only a Summary of changes section.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chriscanin
feat(api): add public getDeviceToken() accessor
848dfd8 
Exposes the device token through the SDK's encrypted storage layer
so external consumers (e.g. the Expo bridge) can read it without
bypassing StorageCipher. Direct SharedPreferences reads break after
the storage encryption change in #585.
chriscanin added a commit to clerk/javascript that referenced this pull request Apr 10, 2026
@chriscanin
fix(expo): use SDK API for client token and delegate sign-out cleanup
2e18123 
- Bump clerk-android to 1.0.11 which handles Client.getSkippingClientId()
  in SignOutService.signOut() (clerk/clerk-android#587)
- Remove our bridge-level getSkippingClientId() calls (now redundant)
- Use Clerk.getDeviceToken() instead of raw SharedPreferences read for
  getClientToken(), fixing compatibility with encrypted storage in 1.0.11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

codex

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@swolfand @chriscanin